Integrate archiveReports with ArchiveMetrics timer #23906
Merged
+223
−8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
caddoo
force-pushed
the
dev-19750-timing-metrics
branch
from
7d18ad1 to
dd9f1d2
Compare
caddoo
force-pushed
the
dev-19750-timing-metrics
branch
from
887b7ba to
f08ed6e
Compare
mneudert
reviewed
Dec 19, 2025
| $segmentObj | ||
| ); | ||
| if ($report) { | ||
| $parameters->setArchiveOnlyReport($report); |
Member
There was a problem hiding this comment.
Is it relevant for the current metric gathering if a single report was requested? For example, when a new custom report is being archived?
Contributor
Author
There was a problem hiding this comment.
I've decided now to skip for individual reports, i've left the condition at this level, is that going to be confusing do you think, that there is a decision here and internally in the timer.
Member
There was a problem hiding this comment.
I did not mean to add any skip-check here, I would actually not. The event handler should take care of not doing work on anything it receives but does not want.
Phrasing my question differently:
Would it make sense to add $report to the event signature? It is part of the
archiveReportssignature, and dropping it feels like a rather arbitrary decision.
Contributor
Author
There was a problem hiding this comment.
This is now used and removed $shouldRecordMetrics
plugins/CoreAdminHome/tests/Integration/ArchiveReportsMetricsTimerTest.php
Outdated
Show resolved
Hide resolved
plugins/CoreAdminHome/tests/Integration/ArchiveReportsMetricsTimerTest.php
Outdated
Show resolved
Hide resolved
plugins/CoreAdminHome/tests/Integration/ArchiveReportsMetricsTimerTest.php
Outdated
Show resolved
Hide resolved
Contributor
|
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
github-actions
bot
added
the
Stale
github-actions
bot
removed
the
Stale
mneudert
approved these changes
Jan 14, 2026
Member
mneudert
left a comment
mneudert
left a comment
There was a problem hiding this comment.
Merging into main PR. Will add any comments (e.g. for the broken CI parts) there.
caddoo
added a commit
that referenced
this pull request
Jan 16, 2026
* Integrate archiveReports with ArchiveMetrics timer * Create event and hook timer into it * Add better typing * Make sure plugin is loaded for integration test * Skip metrics for single-report archiving * Add test-only timer reset helper * Simplify archive loader test GET handling * Add nullable type for instance * cs fix * Add strict types * Allow reports to have metrics recorded
mneudert
pushed a commit
that referenced
this pull request
Jan 16, 2026
* Integrate archiveReports with ArchiveMetrics timer * Create event and hook timer into it * Add better typing * Make sure plugin is loaded for integration test * Skip metrics for single-report archiving * Add test-only timer reset helper * Simplify archive loader test GET handling * Add nullable type for instance * cs fix * Add strict types * Allow reports to have metrics recorded
caddoo
added a commit
that referenced
this pull request
Jan 19, 2026
* Add archiving metrics code with timer * Pass in segment/period objects where possible * Make plugin not installed by default * Add support back in for segment archiving * Don't prematurely filter plugins * Add additional test for when archive PHP not triggered * Update UI Screenshots * Make archiving_metrics.idarchive not null * Store archiving period as numeric id * Store segment hash in archiving metrics * Derive ts_started from stored start time * Use mocks for Timer tests * Move ArchivingMetrics description to lang file * Tighten ArchivingMetrics integration test * Add strict types and license headers * Move context field extraction into writer * Update screenshots * Store done flag as archive name * Add support for report timings * Remove null check * Update index names * Add check to make sure idArchives array is always 1 * Update index name * Switch to date function * Remove date now() function * Add 'type' to $runs * Add clean up task for archive metrics (#23907) * Add clean up task for archive metrics * Add deleted-site purge for archiving metrics * Remove not needed setup/teardown * Add strict types * Update UI screenshot * Integrate archiveReports with ArchiveMetrics timer (#23906) * Integrate archiveReports with ArchiveMetrics timer * Create event and hook timer into it * Add better typing * Make sure plugin is loaded for integration test * Skip metrics for single-report archiving * Add test-only timer reset helper * Simplify archive loader test GET handling * Add nullable type for instance * cs fix * Add strict types * Allow reports to have metrics recorded * Update test to remove mocked 'now' * Remove report from archiving metrics context * Add report to context only * Clean up test * Update documented parameter order * Update tests to show full year <-> day cascading behaviour * Update expected UI test files --------- Co-authored-by: Marc Neudert <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Integrates with this work #23902
Not every path is tested as the timer is tested in the other work.
Checklist
Review